Repository navigation
[release/8.0] Fix: hardening http2 to refuse newline chars on trailers and dynamic HPACK table - #69250
Conversation
…HPACK table (#68743) * Hardening trailers to refuse new line chars * Hardening HPACK dynamic header to refuse new line chars * Http3 request trailers with newline chars test * Added test to exercise newline chars on indexed headers
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
BrennanConroy
added
Servicing-consider
|
Hi @BrennanConroy. Please make sure you've updated the PR description to use the Shiproom Template. Also, make sure this PR is not marked as a draft and is ready-to-merge. To learn more about how to prepare a servicing PR click here. |
|
Hi @BrennanConroy. If this is not a tell-mode PR, please make sure to follow the instructions laid out in the servicing process document. |
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The only finding is a minor, non-blocking test assertion nit.
Review tier: Lite
Findings: None
What changed in this PR
Backports Kestrel hardening to reject CR/LF characters in HTTP/2 trailers and dynamic HPACK values.
Changes:
- Enables newline validation for trailers and dynamic HPACK headers.
- Adds HTTP/2 and HTTP/3 regression coverage.
| File | Description |
|---|---|
src/Servers/Kestrel/test/InMemory.FunctionalTests/Http3/Http3StreamTests.cs |
Tests trailer newline rejection; includes a minor assertion-message nit. |
src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2StreamTests.cs |
Tests dynamic-table newline rejection. |
src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2ConnectionTests.cs |
Adds invalid trailer cases. |
src/Servers/Kestrel/Core/src/Internal/Http2/Http2Connection.cs |
Validates dynamic HPACK header values. |
src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.cs |
Validates request trailer values. |
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
BrennanConroy
added
Servicing-approved
|
Hi @BrennanConroy. This PR was just approved to be included in the upcoming servicing release. Somebody from the @dotnet/aspnet-build team will get it merged when the branches are open. Until then, please make sure all the CI checks pass and the PR is reviewed. |
Remove assertion for ConnectionEndReason in Http2StreamTests.
Backport of #68743 to release/8.0
Fix: hardening http2 to refuse newline chars on trailers and dynamic HPACK table
Description
Kestrel previously skipped validation for CR/LF in two places: request trailers and HPACK dynamic table retrieval. Based on RFC 7540 section 10.3, these values must be treated as malformed. The validation for newline characters has now been enabled along with tests to ensure compliance.
Customer Impact
Invalid headers could show up in logs or application logic processing.
Regression?
Risk
Good test coverage, easy to reason about change
Verification
Packaging changes reviewed?